fix: price a covering projection scan from selectivity, not half - #1107

The defect is real and the model is a genuine improvement over the constant. Two things block it, and the first needs your judgement rather than a patch from me.

The model itself I checked and found sound

I went looking for the failure I expected in a selectivity-scaled cost: dividing by baseSurvival can exceed 1 and price a COVERING projection above the base scan, which is structurally wrong since it reads a subset of columns. You cap it:

scale = sel / baseSurvival;
if (scale > 1.0) scale = 1.0;

So that cannot happen. I also checked the premise the cap rests on: the base scan already prices by projected width (pgcolumnar_projected_width_fraction, four uses in columnar_customscan.c), so a covering projection carries no ADDITIONAL width advantage and the sorted pruning sel captures really is the whole of what it buys. The one-stripe floor is right for the same reason: you cannot read less than one written stripe.

The old constant pricing a 5% range and a 50% range identically is not defensible, and your 0.500 / 0.500 before against 0.050 / 0.500 after is the right pair of numbers to show it.

1. It breaks native_join_runtime_filter, and I think that suite was over-credited

FAIL  projection scan is chosen: got [0] want [1]
FAIL  projection outer is not wrapped: got [1] want [0]

At the cap, projRun == serialRun, so the projection no longer wins on cost and the planner drops it. My reading is that the fixture's restriction is not selective relative to what the heap zone map already prunes, so under an accurate model the projection genuinely offers nothing there. It won before only because of the flat 0.5.

If that is right, the fix is that suite's fixture and not your cost model -- a more selective restriction on the sort key, so the projection earns its place. But I am not going to assert that from reading; it needs the two numbers. sel and baseSurvival for that fixture would settle it, and if sel/baseSurvival is at or above 1 there, the model is telling the truth.

The other reading is that a covering projection should keep some advantage even at cap, in which case the cap is the thing to revisit. I do not believe that, for the width reason above, but you own the change.

Either way this cannot merge red, and I would not want it merged by loosening the arm that caught it.

2. The ledger rows name four majors, not five

9 rows, all 15;16;17;18

Major 19 is missing. Every other row in the file reads 15;16;17;18;19, and the five-major release gate refuses a known check seen on a major its row does not name. CI cannot show you this: ci.yml runs 17 and 18 per PR and only the local matrix adds 19.

This is the same thing that caught all five of your earlier PRs today, and it is not your mistake so much as the tool's: pgc_ledger.py merge writes whatever majors the logs it is given contain and never warns that others are absent. That is #1071 and I have taken it.

Until it is fixed, the rows have to come from five real runs. One caution from doing exactly this an hour ago: do not fan the five majors out in parallel from one source tree. Separate prefixes avoid the install race but they all make into the same build directory and clobber each other, and four of my five loaded a library built for the wrong major while still producing a plausible-looking log. Give each major its own tree.

Your budget and ledger agree at 1275, which is right for the main you branched from. main is now 9caec4c and the count there is 1277, so that needs re-deriving by counting after you rebase, not adjusting by arithmetic.

What I am not asking for

The twins are independent by construction (different tables, row counts and bounds, neither importing the other) and the TDD evidence is red-before-green in both harnesses. That is the part people usually skip and you did not.

The defect is real and the direction is right — a constant that ignores the restriction had to go. But the new price is computed from a selectivity the projection cannot deliver, and for one query class it is now further from the truth than the 0.5 it replaces. Also, the ledger rows will redden PG19, and CI cannot see it.

1. The price follows restrictions the projection cannot prune on

sel = (rel->tuples > 0.0) ? (rel->rows / rel->tuples) : 1.0;

rel->rows is the estimate after every baserestrictinfo clause. The projection prunes on one thing only — p->sortKey[0] — and pgcolumnar_choose_projection offers the path whenever that column appears in some clause:

skips = (p->sortKeyLen > 0 &&
         bms_is_member(p->sortKey[0] - FirstLowInvalidHeapAttributeNumber, restrictCols));

So a query that restricts the sort key not at all in practice and gets its selectivity from another column still qualifies, and is then priced as if the sort order produced that selectivity.

Measured on your own fixture shape, 20,000 rows, stripe_row_limit => 1000, scrambled, projection byk over (k, tag) sorted on k:

A and C are right. B is the problem: the k range is the entire table, so the sort order rules out nothing, and all of the selectivity comes from tag, which the projection cannot prune on.

The planner picks the projection, and it does identical work:

                         projection ON     projection OFF
Columnar Chunk Groups Read:      9                9
Columnar Chunk Groups Removed:  11               11
Buffers: shared hit=           282              282

Same groups read, same buffers. It is in fact slightly worse — Columnar Vector Decodes: 30 against 18, and Rows Removed by Filter: 4990 against 4490 — for a path quoted at 11% of the base.

And this class regresses against the code you are replacing:

truth (identical work)      ratio ~1.0
old constant 0.5            ratio  0.500   <- 2x optimistic
this PR                     ratio  0.111   <- 9x optimistic

The arithmetic is exactly your formula: sel = 10/20000 = 0.0005, floored to 1/20 = 0.05, divided by a base survival of ~0.45, giving 0.111.

The fix is the seam you already have. choose_projection builds restrictCols and tests membership of sortKey[0]. Compute the selectivity from only the clauses that reference that column — clauselist_selectivity over that subset — rather than from rel->rows. Then B gets sel = 1.0, scale clamps to 1.0, and the projection is priced as the base scan, which is what it costs.

Your suite cannot see this, and that is the part worth fixing regardless of how you price it: SQL_TIGHT and SQL_LOOSE both restrict on k alone, so every query in both twins has all of its selectivity in the sort key. One arm with a predicate on a non-sort-key column would pin the property the comment claims — "its run cost follows the restriction's selectivity" — against the case where those two things differ.

2. The ledger rows will redden PG19, and neither CI matrix runs it

The nine new rows claim four majors; every one of the other 1,276 rows claims five:

   1276 rows   15;16;17;18;19
      9 rows   15;16;17;18     <- this PR

PG19 is a covered major, so the gate refuses these checks there. Proved three ways against your own ledger, with a synthesized log in the harness's RESULT format:

PG18 log, your ledger          rc=0   clean
PG19 log, your ledger          rc=1   "not in the ledger: ... (on major 19)" x9
PG19 log, rows widened to ;19  rc=0   clean

ci.yml runs pg: ['15','16','17','18'] and pg: ['17','18']. Neither runs 19, so this PR goes green and the five-major release gate is where it reddens — which is #1071 exactly, and the fifth PR to hit it.

Either run the suite on PG19 and merge that log, or widen the nine rows. I have PG19 on my box and am happy to run it and hand you the rows if that is easier.

3. What I checked and found sound

• The census reconciles. awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l gives 1275, matching the budget. suites_not_covered correctly stays at 249: the suite is both registered and seeded, so it moves neither term.
• pgcolumnar_written_stripe_row_limit is the right function for the floor — the WRITTEN geometry, not the session's GUC. That distinction cost this cost model a whole round once before, and you got it right first time.
• Dividing by baseSurvival is correct reasoning and worth the comment it has: serialRun already carries the heap's pruning, so you have to strip it before applying the projection's, or the discount is taken twice.
• The twins are genuinely independent — different tables, row counts and bounds, neither imports the other, same public EXPLAIN seam.
• The causation mutation (projRun = serialRun * 0.5) reddening both twins for the same got/want is the right control.

One smaller thing, not blocking: the one-stripe floor is applied to sel before the division, so the effective floor on the final fraction is floorFrac / baseSurvival rather than floorFrac. If the intent is "never cheaper than reading one stripe", the clamp belongs on scale. As written it is conservative, so it is a clarity point rather than a bug.

Correcting my own review. I wrote that "the model itself I checked and found sound". That was wrong, and @OffgridwithJD found the half I missed.

What I got wrong

I checked one axis: whether the model can price a covering projection ABOVE the base. It cannot, because of your cap, and I stopped there and called the model sound. I never asked the other question, which is whether the selectivity being credited is selectivity the projection actually delivers.

It is not. Verified at the source rather than from their message:

columnar_customscan.c:1698   skips = (p->sortKeyLen > 0 &&
                                      bms_is_member(p->sortKey[0] - ..., restrictCols))
columnar_customscan.c:2914   sel = rel->rows / rel->tuples

Eligibility is gated on sortKey[0] being among the restriction columns, so the projection prunes on that column alone. But rel->rows is the estimate after every baserestrictinfo. A query restricted loosely on the sort key and tightly on some other column gets credited for pruning it cannot do.

Their measurement on your fixture shape:

A  k BETWEEN 1 AND 1000          ratio 0.050   correct
B  k full range AND tag='rare'   ratio 0.111   9x too cheap
C  k full range AND tag='common' ratio 0.999   correct

B does identical work either way, Chunk Groups Read 9 vs 9 and Buffers 282 vs 282, and is quoted at 11% of the base against a truth near 1.0. So for that class the old constant was 2x wrong and this is 9x wrong. That class regresses, which is the one thing a cost fix must not do.

Their suggested shape is right: derive sel from only the clauses referencing sortKey[0], using the same restrictCols membership choose_projection already computes a few lines up. The information you need is already in the function.

What that changes about my first review

My reading of the native_join_runtime_filter failure was that the fixture had been over-credited by the flat 0.5 and needed a tighter restriction. I would hold that reading loosely now. With sel computed from the wrong clauses, the three failing suites may be reacting to a model that is wrong in both directions at once, and a fixture edit made against it would bake that in. I would fix sel first and re-run the three, then decide whether any fixture still needs changing.

Two of three suites I did not even see, by the way: they attributed projections and projection_update as well, main passing and the PR tree failing on the same build.

Standing

My ledger finding is unchanged and independent: the nine rows read 15;16;17;18 and the five-major gate will refuse them. They reproduced that three ways with a synthesized PG19 log, including the control where widening the rows to ;19 makes it clean.

The TDD and the twins remain the best part of this PR. The defect you identified is real and the constant was indefensible. It is the replacement that needs another pass.

Re-reviewing adversarially against current main, because my earlier findings were measured on a tree that is now 33 commits old and I do not want you acting on a stale claim. Both findings reproduce, and I have added a same-box control this time.

First, something actionable that is not about the code

Your branch predates the CHANGELOG.md union merge driver (#996/#1108), and that changes how you should update it:

merge main INTO the branch   conflicts: CHANGELOG.md  check_ledger_budget.txt  TESTS.md
rebase the branch ONTO main  conflicts:               check_ledger_budget.txt  TESTS.md

CHANGELOG conflicts one way and not the other. Git reads .gitattributes from the tree being merged into, and your head does not carry the driver yet — grep -c 'CHANGELOG.md.*merge=union' gives 0 on your head and 1 on main. So rebase, do not click "Update branch": it saves you a conflict, and the two that remain are derived files whose numbers get re-derived anyway.

Finding 1 reproduces: the price follows restrictions the projection cannot prune on

Rebased onto 6eead59, rebuilt, same fixture shape:

And the work each plan actually does for B:

projection on : Chunk Groups Read: 9   Vector Decodes: 30   Buffers: shared hit=282
projection off: Chunk Groups Read: 9   Vector Decodes: 18   Buffers: shared hit=282

Same groups read, same buffers, and the projection plan decodes more vectors — priced at 11% of the base for work that is, if anything, slightly greater.

The cause is unchanged in the code:

sel = (rel->tuples > 0.0) ? (rel->rows / rel->tuples) : 1.0;

rel->rows is the estimate after every baserestrictinfo clause, while choose_projection offers the path whenever p->sortKey[0] appears in some clause. B's k range covers the whole table, so the sort order rules nothing out and all of the selectivity comes from tag, which the projection cannot prune on.

Finding 2 reproduces, now with a control

                                 #1107 rebased        current main
native_join_runtime_filter       rc=1  2 FAILs        rc=0  0 FAILs
projections                      rc=1  2 FAILs        rc=0  0 FAILs
projection_update                rc=1  1 FAIL         rc=0  0 FAILs

Same box, same PG18, built from each tree in turn — five arms red on the PR and zero on main, all of the shape projection scan is chosen: got [0] want [1]. Where the heap already prunes about as well as the projection would, scale clamps to 1.0, the projection is priced identically to the base, and add_path keeps the incumbent.

Two things I suspected and disproved, so you are not chasing them

The parallel path is not missing the fix. Only one site creates a projection path (columnar_customscan.c:2878), and the partial path never calls choose_projection at all — so there is no second cost site to update. I looked because the diff context made it appear otherwise.

The one-stripe floor is not what makes B cheap. For B, sel = 10/20000 = 0.0005, floored to 1/20 = 0.05, then divided by a base survival of about 0.45, giving 0.111. The floor moves the number up; the division is what makes it too cheap.

Where this leaves it

The defect you found is real, and A and C show the replacement works for the case it was designed for. What is unresolved is which half is wrong when the two disagree, and I still do not think that is mine to decide:

1. the model is right and those three suites were propped up by the constant, in which case they need fixtures where the projection genuinely wins; or
2. sel should come from only the clauses referencing sortKey[0] — the same restrictCols membership choose_projection already computes.

I lean to (2) being necessary regardless, because sel is documented as "the restriction's selectivity" and is not that today. Whether (2) alone restores the three suites I have not measured, and I will not claim it.

The ledger rows still need PG19: ci.yml runs ['15','16','17','18'] and ['17','18'], so neither matrix would catch it and the five-major release gate is where it reddens. I have PG19 here and will run it and hand you the rows once the cost model settles, since the ledger has to be regenerated after that anyway.

The sel fix is right, and it is the hard half. One blocker left and it is mechanical.

What you fixed, verified at source

columnar_customscan.c:1639   foreach(lc, rel->baserestrictinfo)
columnar_customscan.c:1644       pull_varattnos((Node *) ri->clause, rel->relid, &cols);
columnar_customscan.c:1645       if (bms_is_member(sortAttno - FirstLowInvalidHeapAttributeNumber, cols))
columnar_customscan.c:1649   if (no such clause) return 1.0;
columnar_customscan.c:1650   return clauselist_selectivity(root, clauses, ...);

That is the same membership test choose_projection already computes, which is what @OffgridwithJD and I both pointed at, and returning 1.0 when no clause touches the sort key is the right conservative default — no credit for pruning the projection cannot do.

And you added the arm for the case that was 9x too cheap:

check "a non-sort-key restriction does not cheapen a covering projection"

That is the one that matters. The defect you originally identified was real, the flat 0.5 was indefensible, and the replacement now prices the thing it actually prunes on.

The three suites that were failing are green: CI shows 0 failures.

The blocker: the twelve new ledger rows still name four majors

keys added: 12, all in projection_scan_cost, all majors=15;16;17;18

Nineteen is missing, and the gate matches a row only where its majors intersect the run's. Demonstrated rather than asserted — a PG19 leg against this PR's own ledger:

not in the ledger: projection_scan_cost  ...  a non-sort-key restriction does not cheapen a covering projection  (on major 19)
not in the ledger: projection_scan_cost  ...  a tight covering projection is cheaper relative to the base than a loose one  (on major 19)
... 12 in total

Your CI cannot see this and will stay green. ci.yml runs pg: ['15','16','17','18'] and ['17','18'] — no PG19 leg exists there. The refusal happens in the five-major release gate, after merge, on main.

The fix, which is now one command

Since #1071 landed, merge takes several logs at once and warns when it does not get them:

python3 test/pgc_ledger.py merge --ledger test/check_ledger.tsv --date <today> \
    <log-pg15> <log-pg16> <log-pg17> <log-pg18> <log-pg19>

Run projection_scan_cost.sh once per major, merge all five in one call. If you seed from four again, the tool now says so before you commit:

WARNING: 12 row(s) written carrying majors=15;16;17;18, while 1376 other row(s)
         carry 15;16;17;18;19.
         The gate will refuse these on every major they do not name, so this
         reddens on 19.

Five authors hit this before you, including the person who wrote the tool, which is why it is the tool's job to say so now rather than a reviewer's.

Two things I would copy, having been bitten by both this week:

• Freeze the tree before the five runs. I once got four legs green and one red with identical check-name hashes, and it read exactly like a major-specific defect. It was an edit I made while the loop was running.
• Re-derive checks_never_observed_red after the merge, by the budget file's own awk line, on the merged tree. Do not add a delta — that key has been three different correct values on three branches today and none of them survived the merge.

After that

Re-request review and I will run it. Everything substantive is addressed; this is an artefact the branch is missing, not a change of mind about the code.

Both findings are fixed, and I checked them by running the thing rather than by reading
the diff. Approving.

1. The price no longer follows restrictions the projection cannot prune on — verified

pgcolumnar_sortkey_selectivity collects only the baserestrictinfo clauses that
reference the projection's own sort key and prices from those. The part I went looking for
a hole in is the coupling: eligibility tests p->sortKey[0], and pricing has to use the
same column or the two can disagree silently. It does — sortAttnoOut is assigned in the
same block that sets best, and there is only one such block:

1739:  best = pstrdup(p->name);
1740:  bestNcols = p->columnsLen;
1741:  if (sortAttnoOut != NULL)
1742:      *sortAttnoOut = p->sortKey[0];

So there is no path that selects a projection and leaves sortAttno at 0. Measured on my
own fixture — the query shape that was ratio 0.111 when I reported it:

pg18a   -- misattr proj_run=... base_run=... ratio=1.000
pg19a   -- misattr proj_run=327.93 base_run=327.93 ratio=1.000

Exactly 1.000, which is the right answer: the sort key rules out nothing, so there is no
discount to give.

The suite you added pins it as a non-sort-key restriction does not cheapen a covering projection, on its own fixture (prsk, own N, own column names). Run here:

projection_scan_cost.sh on /usr/local/pg18a   12 passed, 0 failed, rc=0
projection_scan_cost.sh on /usr/local/pg19a   12 passed, 0 failed, rc=0

And the arm carries last-red 2026-09-18, so it has actually been observed red rather
than merely added.

2. The PG19 ledger rows — verified, and on the merged tree

All twelve projection_scan_cost rows now read 15;16;17;18;19. The PG19 run is real: I
ran the suite on pg19a myself and its records name major 19, which is the thing CI cannot
tell you because it runs suites on 17 and 18 only.

I checked the numbers against the merge rather than the branch, since two branches cannot
share one total:

merge of pr1107 into main (673895b)   clean, rc=0, no conflict
census re-derived on the merged tree  1377     file states 1377   ✓
duplicate (suite, part, name) keys    0
guard_tests by collection             374      file states 374    ✓

The ledger auto-merged silently, as it does, so I checked it by key rather than trusting
it: zero duplicates.

CI is green on b15fb8a — 14 of 14, builds on all five majors, both pytest halves,
suites on 17 and 18.

One thing left over, filed rather than blocked: #1126

Eligibility and pricing now agree with each other, which is the fix. But both ask does
this clause mention sortKey[0], and mentioning is not the same as being prunable by it.
A single RestrictInfo that ORs a sort-key range with a predicate on another column is
counted whole.

control   WHERE sk BETWEEN 1 AND 2000                   ratio 0.0999
residual  WHERE sk BETWEEN 1 AND 2000 OR kind = 'odd'   ratio 0.1008

The second one prunes nothing:

                                   projection ON    projection OFF
Columnar Usable Skip Predicates:         0                0
Columnar Vectors Skipped:                0                0
Columnar Chunk Groups Read:             20               20   (of 20)
Columnar Vector Decodes:               120               80
Execution Time:                      2.653 ms         1.996 ms

Reads every chunk group, decodes 50% more, finishes 33% slower, quoted at 10% of the base.
The control earns its discount in the same run — Usable Skip Predicates: 2,
Vectors Skipped: 20 — so the fixture distinguishes a real discount from a fabricated one.

Why I am not holding the PR for it. Before this change the constant 0.5 priced that
same query at half the base, which also beats the base and is also chosen. The plan choice
is identical before and after; only the confidence changes. This PR strictly improves the
AND shape and leaves the OR shape where it found it, so blocking would be charging you for
ground you did not lose. The fix direction is in #1126: key on usability as a skip
predicate — the property the executor already computes and prints — rather than on the key
being mentioned.

I will say that my first version of that probe was vacuous and I nearly reported it: with
the range at sk BETWEEN 1 AND 100, both arms sit on the one-stripe floor (20 stripes, so
0.05) and a fabricated discount is indistinguishable from a clamped one. It took raising
the range above the floor to make the question askable.

Approving

Two findings raised, two fixed, both re-proven on a tree I built — including the PG19 arm
that neither CI nor the original claim could reach. The remaining mispricing is real,
narrower than what you fixed, and now has an issue with a reproduction.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

Approving b15fb8a. The blocker is cleared and I re-read the ledger by key rather than trusting the label.

main 1376 rows   #1107 1388 rows
  main keys lost   0
  keys added      12    all projection_scan_cost
  majors          12 of 12 at 15;16;17;18;19     <- was 15;16;17;18
  duplicate keys   0
  census recipe 1377    stated 1377

That is the whole of what I was blocking on, and it is gone. The gate demonstration I posted last round — twelve checks refused (on major 19) — no longer reproduces against this head.

The substance, which was the hard part

pgcolumnar_sortkey_selectivity is still the version I verified: it walks rel->baserestrictinfo, keeps only clauses whose pull_varattnos includes the sort key by the same bms_is_member test choose_projection computes, and returns 1.0 when none do. A projection now gets credit only for pruning it can actually perform.

And the arm for the case that was 9x too cheap is there:

check "a non-sort-key restriction does not cheapen a covering projection"

CI is green on all checks, including the three suites this branch was failing two rounds ago.

Worth saying plainly

You found a real defect — the flat 0.5 was indefensible and nobody else had noticed it. The first replacement was wrong in a way that was genuinely hard to see, because it was wrong in the opposite direction from the bug it fixed, and on a shape the original fixture did not exercise. Two reviewers took three rounds to converge on it. The final version prices the thing the projection actually prunes on, and the suite now covers both directions.

Thanks for staying with it through the ledger seeding as well — that part is pure chore and it is the part that would have broken the release gate silently, on a leg your CI does not run.